Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ci: Add markdownlint, test_converting_readme, and build_docs workflows #247

Merged
merged 4 commits into from
Aug 29, 2023
Merged

ci: Add markdownlint, test_converting_readme, and build_docs workflows #247

merged 4 commits into from
Aug 29, 2023

Conversation

spetrosi
Copy link
Contributor

@spetrosi spetrosi commented Aug 15, 2023 •
edited
Loading

Enhancement: Add markdownlint, test_converting_readme, and build_docs GitHub workflows

Reason:

  • markdownlint runs against markdown files to ensure correct syntax and avoid any issues with converting README.md to HTML
  • test_converting_readme converts README.md > HTML and uploads this test artifact to ensure that conversion works fine
  • build_docs converts README.md > HTML and pushes the result to the docs branch to publish dosc to GitHub pages site
  • Rename commitlint.yml workflow into pr-title-lint for clarity

@spetrosi
Copy link
Contributor Author

I also fix markdownlint issues on README.md and structure it.
After this is merged, you will be able to enable GitHub pages, build on branch docs from the docs/ directory. And then add URL to the page to the repo description like in https://github.com/linux-system-roles/ssh

@spetrosi
Copy link
Contributor Author

Fixing ansible-lint errors in #247

@spetrosi
ci: Add markdownlint, test_converting_readme, and build_docs workflows
886d78a 
* markdownlint runs against README.md to avoid any issues with
  converting it to HTML
* test_converting_readme converts README.md > HTML and uploads this test
  artifact to ensure that conversion works fine
* build_docs converts README.md > HTML and pushes the result to the
  docs branch to publish dosc to GitHub pages site
@spetrosi
Apply latest changes for README conversion
cfe08e3 
* Remove badges from README.md prior to converting to HTML
* Replace Commitlint with PR Title Lint
* Lint all markdown files except for CHANGELOG.md not just README.md
* Use woke form from linux-system-roles
Jakuje
Jakuje reviewed Aug 29, 2023
View reviewed changes
.github/workflows/markdownlint.yml Outdated
Comment on lines 21 to 25
- name: Update pip, git
run: |
set -euxo pipefail
sudo apt update
sudo apt install -y git
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the name does not match the command. I see only git installation here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Jakuje
Jakuje approved these changes Aug 29, 2023
View reviewed changes
Copy link
Collaborator

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am good with the changes (except for the last note in README regarding the sshd_*. It rewrites a lot of documentation though so I would like to hear @mattwillsher opinion too.

README.md Outdated
Comment on lines 143 to 142
* `sshd_...`
#### sshd_
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we lost here the ... so I am not sure how intuitive this is going to be without that. In official documentation we have sshd_<OptionName>. Would using something like this work also here or can this be tweaked somehow to pass the linter?

https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9/html-single/automating_system_administration_by_using_rhel_system_roles/index#sshd-system-role-variables_configuring-secure-communication-by-using-the-ssh-and-sshd-rhel-system-roles

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I missed this because markdownlint removed this automatically. markdownlint does not argue if I put ... or in a backtick. So, I used "sshd_<OptionName>" to be consistent with the official documentation.

mattwillsher
mattwillsher approved these changes Aug 29, 2023
View reviewed changes
Copy link
Member

@mattwillsher mattwillsher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@spetrosi
Copy link
Contributor Author

Please merge

@spetrosi
Copy link
Contributor Author

After this is merged, you will be able to enable GitHub pages, build on branch docs from the docs/ directory. And then add URL to the page to the repo description like, I believe you can keep the link to your ansible-galaxy that's there already and add a link to GitHub pages separated by a comma

@Jakuje Jakuje merged commit 1e308d6 into willshersystems:master Aug 29, 2023
16 checks passed
@Jakuje
Copy link
Collaborator

Jakuje commented Aug 31, 2023

@spetrosi sounds like the docs building failed after meging to master. Can you check what is the issue there or is just needed to allow the pages somewhere in the repo configuration?

@mattwillsher
Copy link
Member

I think it needs a release build to generate the docs(?)

@spetrosi
Copy link
Contributor Author

spetrosi commented Sep 4, 2023

@Jakuje the workflows were configured to run on pushed to main, and ansible-sshd uses master branch. Fixing this in #253 please take a look.

@spetrosi
Copy link
Contributor Author

I think it needs a release build to generate the docs(?)

Now, after master>main rename and after a PR that changes README.md has been merged, the docs are generated on docs branch. You now can enable GitHub pages - build on branch docs from the docs/ directory. And then add URL to the page to the repo description, like in https://github.com/linux-system-roles/ssh.

The next time you create a new release with https://github.com/linux-system-roles/auto-maintenance/blob/main/role-merge-prs.sh it will also generate .README.html within the PR that updates CHANGELOG.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jakuje Jakuje Jakuje approved these changes

@mattwillsher mattwillsher mattwillsher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@spetrosi @Jakuje @mattwillsher